[Fix] Swap package dependency ruff for black and autoflake #757

maxschulz-COL · 2024-09-30T12:29:10Z

Description

Given our continued difficulties to make ruff run on WebAssembly, we decide to swap the usage of ruff via subprocess for using black and autoflake. Using ruff is still the preferred goal, but as long as there is no python API, as well as a WASM compiled version available, we will stick with the current solution.

Note:
ruff was previous used in 2 places for Vizro-AI code string formatting:

inside vizro-ai plot
inside vizro-core pydantic to python (vizro models to python code) conversion.
This PR is mainly to drop the ruff use for these two.

Screenshot

Notice

I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
- I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
- I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
- I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
- I have not referenced individuals, products or companies in any commits, directly or indirectly.
- I have not added data or restricted code in any commits, directly or indirectly.

vizro-ai/changelog.d/20240930_141508_maximilian_schulz_swap_ruff_for_black.md

for more information, see https://pre-commit.ci

huong-li-nguyen

Is our goal to replace ruff completely or just as a mkdocs requirement?

For example there are still some mentions of ruff and we use it e.g. for pydocstyle etc.
Does this not have to be replaced? Or is this fine because it's run via our pre-commit hook?


[tool.ruff]
line-length = 120

[tool.ruff.format]
docstring-code-format = true

[tool.ruff.lint.pydocstyle]
convention = "google"

[tool.ruff.lint.pylint]
max-args = 6

...

lingyielia · 2024-09-30T15:33:21Z

Is our goal to replace ruff completely or just as a mkdocs requirement?

For example there are still some mentions of ruff and we use it e.g. for pydocstyle etc. Does this not have to be replaced? Or is this fine because it's run via our pre-commit hook?
[tool.ruff]
line-length = 120

[tool.ruff.format]
docstring-code-format = true

[tool.ruff.lint.pydocstyle]
convention = "google"

[tool.ruff.lint.pylint]
max-args = 6

...

ruff was previous used in 2 places for Vizro-AI code string formatting:

inside vizro-ai plot
inside vizro-core pydantic to python (vizro models to python code) conversion.

This PR is mainly to drop the ruff use for these two.

huong-li-nguyen · 2024-09-30T15:36:26Z

ruff was previous used in 2 places for Vizro-AI code string formatting:

inside vizro-ai plot

inside vizro-core pydantic to python (vizro models to python code) conversion.

This PR is mainly to drop the ruff use for these two.

Got it! Can we add that to the PR description? :)

antonymilne

A sad but necessary change :(

Please could you give a quick example of how this output is different to what ruff gave before? I guess the fact that none of the unit tests were changed means it's exactly the same for all our example cases?

vizro-core/changelog.d/20240930_141041_maximilian_schulz_swap_ruff_for_black.md

vizro-ai/src/vizro_ai/plot/_response_models.py

vizro-core/src/vizro/models/_base.py

…into tidy/swap_ruff_for_black

for more information, see https://pre-commit.ci

maxschulz-COL · 2024-09-30T16:54:10Z

A sad but necessary change :(

Please could you give a quick example of how this output is different to what ruff gave before? I guess the fact that none of the unit tests were changed means it's exactly the same for all our example cases?

Yes, the magic of unit tests :)

So in general not all code string would end up the same, as we are only formatting with black rules (and they may not 100% match - but seem to for the examples) and we only remove unused imports, and no other formats (which previously was the case for ruff but it looks like it was also unused)

maxschulz-COL · 2024-09-30T16:54:55Z

Is our goal to replace ruff completely or just as a mkdocs requirement?

We are not removing our development dependency and tooling, but the package dependency.

maxschulz-COL added 4 commits September 30, 2024 14:14

Remove ruff and add black and autoflake

db0d1ba

Remove ruff and add black and autoflake for vizro-ai

6343e45

Remove ruff mock

b7128b4

Add dependencies for vizro-ai

04cf742

github-actions bot added the Vizro-AI 🤖 Issue/PR that addresses Vizro-AI package label Sep 30, 2024

maxschulz-COL commented Sep 30, 2024

View reviewed changes

vizro-ai/changelog.d/20240930_141508_maximilian_schulz_swap_ruff_for_black.md Show resolved Hide resolved

maxschulz-COL and others added 4 commits September 30, 2024 15:46

Merge branch 'main' into tidy/swap_ruff_for_black

3130ec6

Fix link

bf9493d

Change comment

3073123

Add changelog

227912f

maxschulz-COL marked this pull request as ready for review September 30, 2024 14:22

maxschulz-COL requested review from stichbury, Joseph-Perkins, antonymilne, huong-li-nguyen and lingyielia as code owners September 30, 2024 14:22

[pre-commit.ci] auto fixes from pre-commit.com hooks

887557c

for more information, see https://pre-commit.ci

lingyielia approved these changes Sep 30, 2024

View reviewed changes

huong-li-nguyen reviewed Sep 30, 2024

View reviewed changes

huong-li-nguyen self-requested a review September 30, 2024 15:36

huong-li-nguyen approved these changes Sep 30, 2024

View reviewed changes

antonymilne approved these changes Sep 30, 2024

View reviewed changes

vizro-core/changelog.d/20240930_141041_maximilian_schulz_swap_ruff_for_black.md Outdated Show resolved Hide resolved

vizro-ai/src/vizro_ai/plot/_response_models.py Outdated Show resolved Hide resolved

vizro-core/src/vizro/models/_base.py Outdated Show resolved Hide resolved

maxschulz-COL and others added 4 commits September 30, 2024 18:46

Merge branch 'main' into tidy/swap_ruff_for_black

727668a

PR comments

a28b5f0

Merge branch 'tidy/swap_ruff_for_black' of github.com:mckinsey/vizro …

7fe70f4

…into tidy/swap_ruff_for_black

[pre-commit.ci] auto fixes from pre-commit.com hooks

341ef27

for more information, see https://pre-commit.ci

maxschulz-COL changed the title ~~[Fix] Swap ruff for black and autoflake~~ [Fix] Swap package dependency ruff for black and autoflake Sep 30, 2024

maxschulz-COL merged commit 88bac23 into main Sep 30, 2024
39 checks passed

maxschulz-COL deleted the tidy/swap_ruff_for_black branch September 30, 2024 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Swap package dependency ruff for black and autoflake #757

[Fix] Swap package dependency ruff for black and autoflake #757

maxschulz-COL commented Sep 30, 2024 •

edited by lingyielia

Loading

huong-li-nguyen left a comment •

edited

Loading

lingyielia commented Sep 30, 2024 •

edited

Loading

huong-li-nguyen commented Sep 30, 2024

antonymilne left a comment •

edited

Loading

maxschulz-COL commented Sep 30, 2024

maxschulz-COL commented Sep 30, 2024

[Fix] Swap package dependency ruff for black and autoflake #757

[Fix] Swap package dependency ruff for black and autoflake #757

Conversation

maxschulz-COL commented Sep 30, 2024 • edited by lingyielia Loading

Description

Screenshot

Notice

huong-li-nguyen left a comment • edited Loading

Choose a reason for hiding this comment

lingyielia commented Sep 30, 2024 • edited Loading

huong-li-nguyen commented Sep 30, 2024

antonymilne left a comment • edited Loading

Choose a reason for hiding this comment

maxschulz-COL commented Sep 30, 2024

maxschulz-COL commented Sep 30, 2024

maxschulz-COL commented Sep 30, 2024 •

edited by lingyielia

Loading

huong-li-nguyen left a comment •

edited

Loading

lingyielia commented Sep 30, 2024 •

edited

Loading

antonymilne left a comment •

edited

Loading